Codecov Report

Attention: Patch coverage is 90.00000% with 26 lines in your changes missing coverage. Please review.

Project coverage is 84.91%. Comparing base (23a49e8) to head (47c9ce7).
Report is 1 commits behind head on potel-base.

✅ All tests successful. No failed tests found.

@@              Coverage Diff               @@
##           potel-base    #4551      +/-   ##
==============================================
+ Coverage       75.55%   84.91%   +9.36%     
==============================================
  Files             144      158      +14     
  Lines           15011    15361     +350     
  Branches         2375     2422      +47     
==============================================
+ Hits            11341    13044    +1703     
+ Misses           2839     1569    -1270     
+ Partials          831      748      -83     

Somewhat better diffable overview of the relevant changes (unfortunately also has some master stuff in it): https://github.com/getsentry/sentry-python/compare/f0ad7ac0a64e9d9aac350558c89513fe4541e21c..d8118e5001ceb2510f4cfc0cf7d9eb625cf77955

The actual changes are:

• making sure we finish the correct span c4ab8cc
• making sure we finish the correct span 2 273a6af
• changing set_data to set_attribute 44e518d
• fixing a pydantic deprecation warning 6dd14ef
• changing type comments to annotations

@antonpirker I am unsure which assertions you are referring to, but in one of the tests the description assertions are no longer needed because they are already checked by the logic that locates the spans in the spans array and assigns them to the correct variable

One question: why have you removed the asserts for span description?

Not sure, @szokeasaurusrex did that in c4ab8cc, maybe because of the op-name-description logic in potel there's now no description?

Is description needed for the insights module?

One question: why have you removed the asserts for span description?

Not sure, @szokeasaurusrex did that in c4ab8cc, maybe because of the op-name-description logic in potel there's now no description?

Is description needed for the insights module?

@sentrivana @antonpirker see my earlier reply. The assertions were made redundant by the logic which selects the spans based on their description

ok, understand.

I brought the asserts back. They are part of the OTel semantic conventions and a couple of asserts more does not make a performance difference and it also makes the tests easier to read and understand what the description should look like.